From 4231b0304d47ef7810f8615e64fda2da093bec4a Mon Sep 17 00:00:00 2001 From: tsteven4 <13596209+tsteven4@users.noreply.github.com> Date: Sat, 23 May 2020 15:36:29 -0600 Subject: [PATCH] fix a few -Wstringop-truncation warnings (#565) * fix -Wstringop-truncation warning in bushnell. * fix -Wstringop-truncation in alantrl. * fix -Wstringop-truncation warning in garmin, and a real error. The real error: In garmin.cc route_hdr_pr would fail to terminate the GPS_SWay rte_ident member if latin1 encoded route name was longer than 255 characters. Subsequently garmin.cc route_write passes the GPS_SWay structure to GPS_Command_Send_Route. GPS_Command_Send_Route may call GPS_A20[01]_Send which may call GPS_D202_Send. GPS_D202_Send assumes that rte_ident is null terminated. The other change only silences the warning, the character array was subsequently terminated anyway. * fix -Wstringop-truncation warning in enigma. These are nonstrings. They are not required to be null terminated. A real reference file caputred from MGL Central 2.0 is added. In it you can see that don't care values are not all filled with a particular value. We fill them with NULLs are a result of zeroing the structure before we write it. You can also see the 1000m offset in altitudes is correct. * correct reference file mode. * fix two strncpy warnings that gcc 9.3.0 doesn't issue. Perhaps it is a gcc bug that these didn't cause warnings. It seems to be related to the fact they were the last member in the structure. In any event, we can copy 1 byte less, we explicitly add a terminator subsequently. --- alan.cc | 11 +++++------ bushnell.cc | 2 +- enigma.cc | 12 ++++++------ garmin.cc | 9 +++++---- reference/mglcentral20.ert | Bin 0 -> 96 bytes reference/mglcentral20.gpx | 19 +++++++++++++++++++ testo.d/enigma.test | 7 +++++++ 7 files changed, 43 insertions(+), 17 deletions(-) create mode 100644 reference/mglcentral20.ert create mode 100644 reference/mglcentral20.gpx diff --git a/alan.cc b/alan.cc index b03143679..1b21c7c57 100644 --- a/alan.cc +++ b/alan.cc @@ -811,24 +811,23 @@ static void trl_track_hdr(const route_head* TL) fatal(MYNAME ": Can't store more than %u tracklogs", MAXTRK); } - if (TL->rte_name != nullptr) { - strncpy(trkhdr[idx].name, CSTRc(TL->rte_name), TRK_NAME_LEN); + if (!TL->rte_name.isEmpty()) { + strncpy(trkhdr[idx].name, CSTRc(TL->rte_name), TRK_NAME_LEN - 1); } if (*(trkhdr[idx].name) == '\0') { sprintf(trkhdr[idx].name, "T%03d", idx); } trkhdr[idx].name[TRK_NAME_LEN-1] = '\0'; - if (TL->rte_desc != nullptr) { - strncpy(trkhdr[idx].comment, CSTRc(TL->rte_desc), TRK_COMMENT_LEN); + if (!TL->rte_desc.isEmpty()) { + strncpy(trkhdr[idx].comment, CSTRc(TL->rte_desc), TRK_COMMENT_LEN - 1); int l = strlen(CSTRc(TL->rte_desc)); if (l < TRK_COMMENT_LEN-1) { - memset(trkhdr[idx].comment + l, ' ', TRK_COMMENT_LEN - l); + memset(trkhdr[idx].comment + l, ' ', TRK_COMMENT_LEN - 1 - l); } } trkhdr[idx].comment[TRK_COMMENT_LEN-1] = '\0'; - trkhdr[idx].comment[TRK_COMMENT_LEN-1] = '\0'; trkhdr[idx].occupied = TRK_USED; trkhdr[idx].totalpt = 0; trkhdr[idx].next = 0; diff --git a/bushnell.cc b/bushnell.cc index dd1f6e1bb..e7b2a44b0 100644 --- a/bushnell.cc +++ b/bushnell.cc @@ -233,7 +233,7 @@ bushnell_write_one(const Waypoint* wpt) gbfputc(bushnell_get_icon_from_name(wpt->icon_descr), file_out); gbfputc(0x01, file_out); // Proximity alarm. 1 == "off", 3 == armed. - strncpy(tbuf, CSTRc(wpt->shortname), sizeof(tbuf)); + strncpy(tbuf, CSTRc(wpt->shortname), sizeof(tbuf) - 1); tbuf[sizeof(tbuf)-1] = 0; gbfwrite(tbuf, sizeof(tbuf), 1, file_out); diff --git a/enigma.cc b/enigma.cc index c9c4ca84c..850f27b4e 100644 --- a/enigma.cc +++ b/enigma.cc @@ -67,10 +67,10 @@ struct enigma_wpt { int32_t longitude; union wpt_data data; uint8_t waypoint_type; - uint8_t shortname_len; - char shortname[6]; - uint8_t longname_len; - char longname[27]; + uint8_t shortname_len; // number of used characters in shortname + char shortname[6]; // ASCII, unused characters are "don't care" values + uint8_t longname_len; // number of used characters in longname + char longname[27]; // ASCII, unused characters are "don't care" values }; static gbfile* file_in, *file_out; @@ -181,11 +181,11 @@ enigma_waypt_disp(const Waypoint* wpt) } if (wpt->shortname != nullptr) { ewpt.shortname_len = (uint8_t) min(6, strlen(CSTRc(wpt->shortname))); - strncpy(ewpt.shortname, CSTRc(wpt->shortname), 6); + memcpy(ewpt.shortname, CSTRc(wpt->shortname), ewpt.shortname_len); } if (wpt->description != nullptr) { ewpt.longname_len = (uint8_t) min(27, strlen(CSTRc(wpt->description))); - strncpy(ewpt.longname, CSTRc(wpt->description), 27); + memcpy(ewpt.longname, CSTRc(wpt->description), ewpt.longname_len); } gbfwrite(&ewpt, sizeof(ewpt), 1, file_out); } diff --git a/garmin.cc b/garmin.cc index 8e0f6b4ca..1803d5ce1 100644 --- a/garmin.cc +++ b/garmin.cc @@ -1053,7 +1053,8 @@ route_hdr_pr(const route_head* rte) (*cur_tx_routelist_entry)->isrte = 1; if (!rte->rte_name.isEmpty()) { strncpy((*cur_tx_routelist_entry)->rte_ident, CSTRc(rte->rte_name), - sizeof((*cur_tx_routelist_entry)->rte_ident)); + sizeof((*cur_tx_routelist_entry)->rte_ident) - 1); + (*cur_tx_routelist_entry)->rte_ident[sizeof((*cur_tx_routelist_entry)->rte_ident) - 1] = 0; } } @@ -1103,7 +1104,7 @@ route_waypt_pr(const Waypoint* wpt) if (wpt->description.isEmpty()) { rte->cmnt[0] = 0; } else { - strncpy(rte->cmnt, CSTR(wpt->description), sizeof(rte->cmnt)); + strncpy(rte->cmnt, CSTR(wpt->description), sizeof(rte->cmnt) - 1); rte->cmnt[sizeof(rte->cmnt)-1] = 0; } cur_tx_routelist_entry++; @@ -1130,7 +1131,7 @@ track_hdr_pr(const route_head* trk_head) { (*cur_tx_tracklist_entry)->ishdr = true; if (!trk_head->rte_name.isEmpty()) { - strncpy((*cur_tx_tracklist_entry)->trk_ident, CSTRc(trk_head->rte_name), sizeof((*cur_tx_tracklist_entry)->trk_ident)); + strncpy((*cur_tx_tracklist_entry)->trk_ident, CSTRc(trk_head->rte_name), sizeof((*cur_tx_tracklist_entry)->trk_ident) - 1); (*cur_tx_tracklist_entry)->trk_ident[sizeof((*cur_tx_tracklist_entry)->trk_ident)-1] = 0; } else { sprintf((*cur_tx_tracklist_entry)->trk_ident, "TRACK%02d", my_track_count); @@ -1147,7 +1148,7 @@ track_waypt_pr(const Waypoint* wpt) (*cur_tx_tracklist_entry)->alt = (wpt->altitude != unknown_alt) ? wpt->altitude : 1e25; (*cur_tx_tracklist_entry)->Time = wpt->GetCreationTime().toTime_t(); if (!wpt->shortname.isEmpty()) { - strncpy((*cur_tx_tracklist_entry)->trk_ident, CSTRc(wpt->shortname), sizeof((*cur_tx_tracklist_entry)->trk_ident)); + strncpy((*cur_tx_tracklist_entry)->trk_ident, CSTRc(wpt->shortname), sizeof((*cur_tx_tracklist_entry)->trk_ident) - 1); (*cur_tx_tracklist_entry)->trk_ident[sizeof((*cur_tx_tracklist_entry)->trk_ident)-1] = 0; } (*cur_tx_tracklist_entry)->tnew = wpt->wpt_flags.new_trkseg; diff --git a/reference/mglcentral20.ert b/reference/mglcentral20.ert new file mode 100644 index 0000000000000000000000000000000000000000..53fb3288f255fca1cbecca48c80fcb402d1ea596 GIT binary patch literal 96 zcmZ4YBbVXDpL_o%NH8!kvUoeWgqoJ~IQfVAxVQ!>_=fs3RR3pcDfG6HvXEvuU|>kiWBcq=K)1s9%U9ntA{|?H+~z literal 0 HcmV?d00001 diff --git a/reference/mglcentral20.gpx b/reference/mglcentral20.gpx new file mode 100644 index 000000000..c834714b5 --- /dev/null +++ b/reference/mglcentral20.gpx @@ -0,0 +1,19 @@ + + + + + + + 1611.782 + KBDU + BOULDER MUNI + BOULDER MUNI + + + 1729.130 + KBJC + ROCKY MOUNTAIN METROPOLITAN + ROCKY MOUNTAIN METROPOLITAN + + + diff --git a/testo.d/enigma.test b/testo.d/enigma.test index 3497127bd..ce58f0420 100644 --- a/testo.d/enigma.test +++ b/testo.d/enigma.test @@ -6,3 +6,10 @@ gpsbabel -i gpx -f ${REFERENCE}/enigma.gpx -o enigma -F ${TMPDIR}/enigma.ert compare ${REFERENCE}/enigma-gpsb.ert ${TMPDIR}/enigma.ert gpsbabel -i enigma -f ${REFERENCE}/enigma-pfms.ert -o gpx -F ${TMPDIR}/enigma.gpx compare ${REFERENCE}/enigma.gpx ${TMPDIR}/enigma.gpx + +# mglcentral20.ert was produced by MGL Central version 2.0. +# don't care values in character string are evident, +# which makes it impossible to compare "identical" enigma files. +gpsbabel -i enigma -f ${REFERENCE}/mglcentral20.ert -o gpx -F ${TMPDIR}/mglcentral20.gpx +compare ${REFERENCE}/mglcentral20.gpx ${TMPDIR}/mglcentral20.gpx + -- 2.30.2